-
Notifications
You must be signed in to change notification settings - Fork 376
bug(test-cli-consume): fix bpos in hive ruleset #1758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
spencer-tb
wants to merge
14
commits into
ethereum:eips/amsterdam/eip-7928
Choose a base branch
from
spencer-tb:bug/consume-amsterdam-bpo-config
base: eips/amsterdam/eip-7928
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
bug(test-cli-consume): fix bpos in hive ruleset #1758
spencer-tb
wants to merge
14
commits into
ethereum:eips/amsterdam/eip-7928
from
spencer-tb:bug/consume-amsterdam-bpo-config
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- bal -> block_access_list; re-add custom rlp encoding for block access list - bytes to uint - move away from method-style - Update EIP-7928 implementation: system contracts at index 0, migrate to RLP - System contracts (parent hash, beacon root) now use block_access_index 0 - Transactions use block_access_index 1 to len(transactions) - Post-execution changes use block_access_index len(transactions) + 1 - Migrated from SSZ to RLP encoding as per updated EIP-7928 spec - Updated all tests to match new API and structure - Replaced tx_index with block_access_index throughout codebase - add system contract logic - add markdown docstrings - update BAL format; address comments - ssz encoding and bal validation - six ssz types - bal tests - balspecs fix: do not track setting empty code to a new account (ethereum#19) fix: track implicit SLOAD within SSTORE for OOG cases (ethereum#18) refactor: Put back explicit acct tracking outside 7702 delegation path (ethereum#17) fix non-tracked 7702 authority for invalid delegations (ethereum#16) * fix non-tracked 7702 authority for invalid delegations * fix: lint issues * fix: track delegation target when loaded as call target * fix: track delegation target when loaded as call target from call opcodes * chore: fix issues with documentation generation Fix self-destruct cases with pre-execution balance cache / tracking * fix self-destruct implementation * fix self-destruct tracking balance * fix it in the bal finalization by filtering * add balance reset and fix tests * simplify pre-balance tracking not using snapshots fix duplicated code entries for in transaction self destruct fix self destruct in same transaction bug fix call/delagate call tracking bug fix zero-value transfer tracking (ethereum#6) * fix zero-value transfer tracking * fix reverted frame tracking * rename variables * fix missing addresses bug * fix: docs run & move imports to top of file refactor: move rlp_utils to block_access_lists; bal -> block_access_lists Some remaining fixes due to large refactor in `forks/osaka`: - Move BALs from amsterdam -> forks/amsterdam - rename: build -> build_block_access_list - fix docc issues move state change tracker to State correct system contract addresses Fixes to communicate with BALs EEST branch: - fix(bal): Initialize the state tracker before system contract calls - We were missing system contract calls to beacon roots and history contracts. This change initializes the state tracker before system contract calls and passes the tracker to these calls if post-Amsterdam. - fix(docs): Fix issues with toxenvs: lint, doc, json_infra - fix(t8n): Only initialize the bal_change_tracker for amsterdam - feat(fork criteria): Index upcoming forks for better ordering / fix issues - chore(forks): Fix issues from lint after rebase with Osaka latest - fix(setuptools): Update packages to include amsterdam - chore(lint): Fix 'tox -e static' issues - Fix bug in tracker Manually cherry-picked from e72991b Author: nerolation - chore(tests): Attempt to resolve issues with CI tests - chore(lint): fix issues from running ``tox -e static`` locally - refactor(bal): Send BAL as a list over t8n tool - fix(amsterdam): Add change tracker to state test in t8n - chore(lint,tests): Fix tests after moving bal from osaka -> amsterdam - chore(forks): Move bals from Osaka to Amsterdam - chore(lint): Fix lint issues - refactor(bal): Send the full bal object and bal_hash over t8n - If we send the full object over JSON, we can model_validate() on ESST. - If we send the hash, once we fill the pydantic model, we can get the rlp and the hash and validate that our objects match while only really validating the parts of the BAL we are interested in for each test. - chore: point to working eest branch - chore(bals): Remove unused SSZ utils.py The SSZ implementation is no longer needed as we are now using RLP - refactor(bals): Clean up BAL module types and imports - Bytes -> Bytes32 type for storage slots - Remove unused imports / fix imports / fix linting - Update function signatures to match tracker - fix(bals-tx-index): Track bal indexes in t8n Keep track of BAL index state in t8n
* fix(specs): Fix zero value withdrawals BAL tracking * docs(specs): rename 'finalize' to 'normalize' in comments * docs(specs): remove reference to uint128 for balance tracking --------- Co-authored-by: Toni Wahrstätter <[email protected]> Co-authored-by: Toni Wahrstätter <[email protected]>
refactor(tests): Prevent skips by splitting tests appropriately fix(tests): Use valid inputs to precompile tests chore(tests): linting fixes feat(tests): EIP-7928 test_bal_storage_write_read_cross_frame feat(tests): EIP-7928 test_bal_storage_write_read_same_frame feat(tests): EIP-7928 test_bal_nonexistent_account_access feat(tests): EIP-7928 test_bal_nonexistent_value_transfer feat(tests): EIP-7928 test_bal_precompiles feat(tests): EIP-7928 test_bal_withdrawal_to_coinbase_empty_block feat(tests): EIP-7928 test_bal_withdrawal_to_coinbase feat(tests): EIP-7928 test_bal_withdrawal_largest_amount feat(tests): EIP-7928 test_bal_withdrawal_to_precompiles fix(tests): expectation for nonexistent account in post fix(specs,tests): Fix withdrawal tests for BALs issue with idx==0 - `self.txs.successfully_parsed` is a list of transaction indexes, not transactions. The "if tx" check here would then check `if 0` which parses as a boolean ``False``. This means we would skip counting the tx if index=0 was successful. - Fixes some test expectations where `post_code` was being checked instead of ``new_code``. feat(tests): EIP-7928 test_bal_zero_withdrawal feat(tests): EIP-7928 test_bal_withdrawal_and_new_contract feat(tests): EIP-7928 test_bal_withdrawal_and_selfdestruct feat(tests): EIP-7928 test_bal_multiple_withdrawals_same_address feat(tests): EIP-7928 withdrawal_and_value_transfer_same_address feat(tests): EIP-7928 withdrawal_and_state_access_same_account feat(tests): EIP-7928 test_bal_withdrawal_no_evm_execution feat(tests): EIP-7928 test_bal_withdrawal_to_nonexistent_account feat(tests): EIP-7928 test_bal_withdrawal_empty_block feat(tests): EIP-7928 test_bal_withdrawal_with_transaction feat(tests): EIP-7928 coinbase
…1743) - Perform a similar check to balance changes and other tracker methods and keep only the last write.
Contributor
Author
|
Set to draft for now as would like to wait for other EL to respond in STEEL discord. |
Contributor
|
These look like good changes to me. We can wait until there's consensus here but looks good on my end. |
4388bef to
0395133
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-test-cli-consume
Area: Tests Consume CLI—pytest command (eg. `p/t/s/e/cli/pytest_commands/consume.py`)
C-bug
Category: this is a bug, deviation, or other problem
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗒️ Description
Enables BPO configs for forks that occur after them such as Amsterdam. This aims to fix the blob related tests on the most recent BAL run on Hive CI: https://hive.ethpandaops.io/#/group/bal
As Amsterdam is based on top of Osaka within
forks.py, the BPO hive variables are not included. This leads to chain spec issues for EL clients due to there ordering of forks, when all the timestamps are set to 0. This PR fixes the latter.Additionally adds BPO5 and a transition fork from BPO5 to Amsterdam. This goes off of the assumption that we will have 5 BPOs on Osaka mainnet which is currently planned. Note BPO5 is added here for chain spec issues in EL clients.
🔗 Related Issues or PRs
N/A.
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.